Skip to content

Conversation

@dpaoliello
Copy link
Contributor

@dpaoliello dpaoliello commented Nov 18, 2024

Enables xs and tlb-rmi target features to be enabled via -march.

@llvmbot llvmbot added clang Clang issues not falling into any other category backend:AArch64 clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 18, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 18, 2024

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Daniel Paoliello (dpaoliello)

Changes

Enables xs and tlb-rmi target features to be enabled via -march and adds new target defines for them in Clang.


Full diff: https://github.com/llvm/llvm-project/pull/116707.diff

4 Files Affected:

  • (modified) clang/lib/Basic/Targets/AArch64.cpp (+12)
  • (modified) clang/lib/Basic/Targets/AArch64.h (+2)
  • (modified) clang/test/Preprocessor/aarch64-target-features.c (+8)
  • (modified) llvm/lib/Target/AArch64/AArch64Features.td (+5-5)
diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
index eb8a3ada034482..420b8ad7a01f09 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -635,6 +635,12 @@ void AArch64TargetInfo::getTargetDefines(const LangOptions &Opts,
   if (HasGCS)
     Builder.defineMacro("__ARM_FEATURE_GCS", "1");
 
+  if (HasXS)
+    Builder.defineMacro("__ARM_FEATURE_XS", "1");
+
+  if (HasTLBRmi)
+    Builder.defineMacro("__ARM_FEATURE_TLB_RMI", "1");
+
   if (*ArchInfo == llvm::AArch64::ARMV8_1A)
     getTargetDefinesARMV81A(Opts, Builder);
   else if (*ArchInfo == llvm::AArch64::ARMV8_2A)
@@ -790,6 +796,8 @@ bool AArch64TargetInfo::hasFeature(StringRef Feature) const {
       .Cases("ls64", "ls64_v", "ls64_accdata", HasLS64)
       .Case("wfxt", HasWFxT)
       .Case("rcpc3", HasRCPC3)
+      .Case("xs", HasXS)
+      .Case("tlb-rmi", HasTLBRmi)
       .Default(false);
 }
 
@@ -1102,6 +1110,10 @@ bool AArch64TargetInfo::handleTargetFeatures(std::vector<std::string> &Features,
       HasPAuthLR = true;
       HasPAuth = true;
     }
+    if (Feature == "+xs")
+      HasXS = true;
+    if (Feature == "+tlb-rmi")
+      HasTLBRmi = true;
   }
 
   // Check features that are manually disabled by command line options.
diff --git a/clang/lib/Basic/Targets/AArch64.h b/clang/lib/Basic/Targets/AArch64.h
index 4c25bdb5bb16df..47149a74a22f26 100644
--- a/clang/lib/Basic/Targets/AArch64.h
+++ b/clang/lib/Basic/Targets/AArch64.h
@@ -118,6 +118,8 @@ class LLVM_LIBRARY_VISIBILITY AArch64TargetInfo : public TargetInfo {
   bool HasRCPC3 = false;
   bool HasSMEFA64 = false;
   bool HasPAuthLR = false;
+  bool HasXS = false;
+  bool HasTLBRmi = false;
 
   const llvm::AArch64::ArchInfo *ArchInfo = &llvm::AArch64::ARMV8A;
 
diff --git a/clang/test/Preprocessor/aarch64-target-features.c b/clang/test/Preprocessor/aarch64-target-features.c
index 037a3e186ee559..be26d0d300f94b 100644
--- a/clang/test/Preprocessor/aarch64-target-features.c
+++ b/clang/test/Preprocessor/aarch64-target-features.c
@@ -738,3 +738,11 @@
 // CHECK-SMEB16B16: __ARM_FEATURE_SME2 1
 // CHECK-SMEB16B16: __ARM_FEATURE_SME_B16B16 1
 // CHECK-SMEB16B16: __ARM_FEATURE_SVE_B16B16 1
+
+// ================== Check Armv8.7-A limited-TLB-maintenance instruction.
+// RUN: %clang -target aarch64 -march=armv8a+xs -x c -E -dM %s -o - | FileCheck -check-prefix=CHECK-XS %s
+// CHECK-XS: __ARM_FEATURE_XS 1
+
+// ================== Check Armv8.4-A TLB Range and Maintenance instructions.
+// RUN: %clang -target aarch64 -march=armv8a+tlb-rmi -x c -E -dM %s -o - | FileCheck -check-prefix=CHECK-TLB-RMI %s
+// CHECK-TLB-RMI: __ARM_FEATURE_TLB_RMI 1
diff --git a/llvm/lib/Target/AArch64/AArch64Features.td b/llvm/lib/Target/AArch64/AArch64Features.td
index 088de4328a198d..189b0646a97085 100644
--- a/llvm/lib/Target/AArch64/AArch64Features.td
+++ b/llvm/lib/Target/AArch64/AArch64Features.td
@@ -227,7 +227,7 @@ def FeatureAM : Extension<"am", "AM", "FEAT_AMUv1",
 def FeatureSEL2 : Extension<"sel2", "SEL2", "FEAT_SEL2",
   "Enable Armv8.4-A Secure Exception Level 2 extension">;
 
-def FeatureTLB_RMI : Extension<"tlb-rmi", "TLB_RMI",
+def FeatureTLB_RMI : ExtensionWithMArch<"tlb-rmi", "TLB_RMI",
   "FEAT_TLBIOS, FEAT_TLBIRANGE",
   "Enable Armv8.4-A TLB Range and Maintenance instructions">;
 
@@ -296,7 +296,7 @@ def FeatureEnhancedCounterVirtualization :
 //  Armv8.7 Architecture Extensions
 //===----------------------------------------------------------------------===//
 
-def FeatureXS : Extension<"xs", "XS", "FEAT_XS",
+def FeatureXS : ExtensionWithMArch<"xs", "XS", "FEAT_XS",
   "Enable Armv8.7-A limited-TLB-maintenance instruction">;
 
 def FeatureWFxT : ExtensionWithMArch<"wfxt", "WFxT", "FEAT_WFxT",
@@ -836,7 +836,7 @@ def HasV8_4aOps : Architecture64<8, 4, "a", "v8.4a",
   [HasV8_3aOps, FeatureDotProd, FeatureNV, FeatureMPAM, FeatureDIT,
     FeatureTRACEV8_4, FeatureAM, FeatureSEL2, FeatureTLB_RMI, FeatureFlagM,
     FeatureRCPC_IMMO, FeatureLSE2],
-  !listconcat(HasV8_3aOps.DefaultExts, [FeatureDotProd, FeatureDIT, FeatureFlagM])>;
+  !listconcat(HasV8_3aOps.DefaultExts, [FeatureDotProd, FeatureDIT, FeatureTLB_RMI, FeatureFlagM])>;
 def HasV8_5aOps : Architecture64<8, 5, "a", "v8.5a",
   [HasV8_4aOps, FeatureAltFPCmp, FeatureFRInt3264, FeatureSpecRestrict,
     FeatureSB, FeaturePredRes, FeatureCacheDeepPersist,
@@ -848,7 +848,7 @@ def HasV8_6aOps : Architecture64<8, 6, "a", "v8.6a",
   !listconcat(HasV8_5aOps.DefaultExts, [FeatureBF16, FeatureMatMulInt8])>;
 def HasV8_7aOps : Architecture64<8, 7, "a", "v8.7a",
   [HasV8_6aOps, FeatureXS, FeatureWFxT, FeatureHCX, FeatureSPE_EEF],
-  !listconcat(HasV8_6aOps.DefaultExts, [FeatureWFxT])>;
+  !listconcat(HasV8_6aOps.DefaultExts, [FeatureXS, FeatureWFxT])>;
 def HasV8_8aOps : Architecture64<8, 8, "a", "v8.8a",
   [HasV8_7aOps, FeatureHBC, FeatureMOPS, FeatureNMI],
   !listconcat(HasV8_7aOps.DefaultExts, [FeatureMOPS, FeatureHBC])>;
@@ -866,7 +866,7 @@ def HasV9_1aOps : Architecture64<9, 1, "a", "v9.1a",
   !listconcat(HasV9_0aOps.DefaultExts, [FeatureBF16, FeatureMatMulInt8, FeatureRME])>;
 def HasV9_2aOps : Architecture64<9, 2, "a", "v9.2a",
   [HasV8_7aOps, HasV9_1aOps],
-  !listconcat(HasV9_1aOps.DefaultExts, [FeatureMEC, FeatureWFxT])>;
+  !listconcat(HasV9_1aOps.DefaultExts, [FeatureMEC, FeatureXS, FeatureWFxT])>;
 def HasV9_3aOps : Architecture64<9, 3, "a", "v9.3a",
   [HasV8_8aOps, HasV9_2aOps],
   !listconcat(HasV9_2aOps.DefaultExts, [FeatureMOPS, FeatureHBC])>;

@llvmbot
Copy link
Member

llvmbot commented Nov 18, 2024

@llvm/pr-subscribers-backend-aarch64

Author: Daniel Paoliello (dpaoliello)

Changes

Enables xs and tlb-rmi target features to be enabled via -march and adds new target defines for them in Clang.


Full diff: https://github.com/llvm/llvm-project/pull/116707.diff

4 Files Affected:

  • (modified) clang/lib/Basic/Targets/AArch64.cpp (+12)
  • (modified) clang/lib/Basic/Targets/AArch64.h (+2)
  • (modified) clang/test/Preprocessor/aarch64-target-features.c (+8)
  • (modified) llvm/lib/Target/AArch64/AArch64Features.td (+5-5)
diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
index eb8a3ada034482..420b8ad7a01f09 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -635,6 +635,12 @@ void AArch64TargetInfo::getTargetDefines(const LangOptions &Opts,
   if (HasGCS)
     Builder.defineMacro("__ARM_FEATURE_GCS", "1");
 
+  if (HasXS)
+    Builder.defineMacro("__ARM_FEATURE_XS", "1");
+
+  if (HasTLBRmi)
+    Builder.defineMacro("__ARM_FEATURE_TLB_RMI", "1");
+
   if (*ArchInfo == llvm::AArch64::ARMV8_1A)
     getTargetDefinesARMV81A(Opts, Builder);
   else if (*ArchInfo == llvm::AArch64::ARMV8_2A)
@@ -790,6 +796,8 @@ bool AArch64TargetInfo::hasFeature(StringRef Feature) const {
       .Cases("ls64", "ls64_v", "ls64_accdata", HasLS64)
       .Case("wfxt", HasWFxT)
       .Case("rcpc3", HasRCPC3)
+      .Case("xs", HasXS)
+      .Case("tlb-rmi", HasTLBRmi)
       .Default(false);
 }
 
@@ -1102,6 +1110,10 @@ bool AArch64TargetInfo::handleTargetFeatures(std::vector<std::string> &Features,
       HasPAuthLR = true;
       HasPAuth = true;
     }
+    if (Feature == "+xs")
+      HasXS = true;
+    if (Feature == "+tlb-rmi")
+      HasTLBRmi = true;
   }
 
   // Check features that are manually disabled by command line options.
diff --git a/clang/lib/Basic/Targets/AArch64.h b/clang/lib/Basic/Targets/AArch64.h
index 4c25bdb5bb16df..47149a74a22f26 100644
--- a/clang/lib/Basic/Targets/AArch64.h
+++ b/clang/lib/Basic/Targets/AArch64.h
@@ -118,6 +118,8 @@ class LLVM_LIBRARY_VISIBILITY AArch64TargetInfo : public TargetInfo {
   bool HasRCPC3 = false;
   bool HasSMEFA64 = false;
   bool HasPAuthLR = false;
+  bool HasXS = false;
+  bool HasTLBRmi = false;
 
   const llvm::AArch64::ArchInfo *ArchInfo = &llvm::AArch64::ARMV8A;
 
diff --git a/clang/test/Preprocessor/aarch64-target-features.c b/clang/test/Preprocessor/aarch64-target-features.c
index 037a3e186ee559..be26d0d300f94b 100644
--- a/clang/test/Preprocessor/aarch64-target-features.c
+++ b/clang/test/Preprocessor/aarch64-target-features.c
@@ -738,3 +738,11 @@
 // CHECK-SMEB16B16: __ARM_FEATURE_SME2 1
 // CHECK-SMEB16B16: __ARM_FEATURE_SME_B16B16 1
 // CHECK-SMEB16B16: __ARM_FEATURE_SVE_B16B16 1
+
+// ================== Check Armv8.7-A limited-TLB-maintenance instruction.
+// RUN: %clang -target aarch64 -march=armv8a+xs -x c -E -dM %s -o - | FileCheck -check-prefix=CHECK-XS %s
+// CHECK-XS: __ARM_FEATURE_XS 1
+
+// ================== Check Armv8.4-A TLB Range and Maintenance instructions.
+// RUN: %clang -target aarch64 -march=armv8a+tlb-rmi -x c -E -dM %s -o - | FileCheck -check-prefix=CHECK-TLB-RMI %s
+// CHECK-TLB-RMI: __ARM_FEATURE_TLB_RMI 1
diff --git a/llvm/lib/Target/AArch64/AArch64Features.td b/llvm/lib/Target/AArch64/AArch64Features.td
index 088de4328a198d..189b0646a97085 100644
--- a/llvm/lib/Target/AArch64/AArch64Features.td
+++ b/llvm/lib/Target/AArch64/AArch64Features.td
@@ -227,7 +227,7 @@ def FeatureAM : Extension<"am", "AM", "FEAT_AMUv1",
 def FeatureSEL2 : Extension<"sel2", "SEL2", "FEAT_SEL2",
   "Enable Armv8.4-A Secure Exception Level 2 extension">;
 
-def FeatureTLB_RMI : Extension<"tlb-rmi", "TLB_RMI",
+def FeatureTLB_RMI : ExtensionWithMArch<"tlb-rmi", "TLB_RMI",
   "FEAT_TLBIOS, FEAT_TLBIRANGE",
   "Enable Armv8.4-A TLB Range and Maintenance instructions">;
 
@@ -296,7 +296,7 @@ def FeatureEnhancedCounterVirtualization :
 //  Armv8.7 Architecture Extensions
 //===----------------------------------------------------------------------===//
 
-def FeatureXS : Extension<"xs", "XS", "FEAT_XS",
+def FeatureXS : ExtensionWithMArch<"xs", "XS", "FEAT_XS",
   "Enable Armv8.7-A limited-TLB-maintenance instruction">;
 
 def FeatureWFxT : ExtensionWithMArch<"wfxt", "WFxT", "FEAT_WFxT",
@@ -836,7 +836,7 @@ def HasV8_4aOps : Architecture64<8, 4, "a", "v8.4a",
   [HasV8_3aOps, FeatureDotProd, FeatureNV, FeatureMPAM, FeatureDIT,
     FeatureTRACEV8_4, FeatureAM, FeatureSEL2, FeatureTLB_RMI, FeatureFlagM,
     FeatureRCPC_IMMO, FeatureLSE2],
-  !listconcat(HasV8_3aOps.DefaultExts, [FeatureDotProd, FeatureDIT, FeatureFlagM])>;
+  !listconcat(HasV8_3aOps.DefaultExts, [FeatureDotProd, FeatureDIT, FeatureTLB_RMI, FeatureFlagM])>;
 def HasV8_5aOps : Architecture64<8, 5, "a", "v8.5a",
   [HasV8_4aOps, FeatureAltFPCmp, FeatureFRInt3264, FeatureSpecRestrict,
     FeatureSB, FeaturePredRes, FeatureCacheDeepPersist,
@@ -848,7 +848,7 @@ def HasV8_6aOps : Architecture64<8, 6, "a", "v8.6a",
   !listconcat(HasV8_5aOps.DefaultExts, [FeatureBF16, FeatureMatMulInt8])>;
 def HasV8_7aOps : Architecture64<8, 7, "a", "v8.7a",
   [HasV8_6aOps, FeatureXS, FeatureWFxT, FeatureHCX, FeatureSPE_EEF],
-  !listconcat(HasV8_6aOps.DefaultExts, [FeatureWFxT])>;
+  !listconcat(HasV8_6aOps.DefaultExts, [FeatureXS, FeatureWFxT])>;
 def HasV8_8aOps : Architecture64<8, 8, "a", "v8.8a",
   [HasV8_7aOps, FeatureHBC, FeatureMOPS, FeatureNMI],
   !listconcat(HasV8_7aOps.DefaultExts, [FeatureMOPS, FeatureHBC])>;
@@ -866,7 +866,7 @@ def HasV9_1aOps : Architecture64<9, 1, "a", "v9.1a",
   !listconcat(HasV9_0aOps.DefaultExts, [FeatureBF16, FeatureMatMulInt8, FeatureRME])>;
 def HasV9_2aOps : Architecture64<9, 2, "a", "v9.2a",
   [HasV8_7aOps, HasV9_1aOps],
-  !listconcat(HasV9_1aOps.DefaultExts, [FeatureMEC, FeatureWFxT])>;
+  !listconcat(HasV9_1aOps.DefaultExts, [FeatureMEC, FeatureXS, FeatureWFxT])>;
 def HasV9_3aOps : Architecture64<9, 3, "a", "v9.3a",
   [HasV8_8aOps, HasV9_2aOps],
   !listconcat(HasV9_2aOps.DefaultExts, [FeatureMOPS, FeatureHBC])>;

Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__ARM_FEATURE_* defines are, as far as I know, supposed to be defined in the ACLE. So there needs to be an issue/PR for the ACLE spec. Once Arm reviews that, we can merge the corresponding patches here.

@dpaoliello
Copy link
Contributor Author

__ARM_FEATURE_* defines are, as far as I know, supposed to be defined in the ACLE. So there needs to be an issue/PR for the ACLE spec. Once Arm reviews that, we can merge the corresponding patches here.

Ah, interesting, any objection to not adding the detection macros?

@davemgreen
Copy link
Collaborator

Could you give more details about why you would want these added? As far as I understand they are mandatory features for 8.4 and 8.7, and would usually be added via -march=armv8.4-a for example. We try to keep the options between GCC and clang the same, and GCC doesn't seem to support either https://godbolt.org/z/TTnq48W81.

@dpaoliello
Copy link
Contributor Author

Could you give more details about why you would want these added?

These instructions are being used in the hand-written assembly for a hypervisor. The hypervisor will check at runtime if the instructions are available on the current CPU before calling this code.

@efriedma-quic
Copy link
Collaborator

In assembly, you can override the current architecture with a .arch directive.

@efriedma-quic
Copy link
Collaborator

Ah, interesting, any objection to not adding the detection macros?

For the feature names, I think we also try to coordinate with Arm to define the names. (Maybe also in the ACLE? Not sure.) So I don't object to splitting the patch, but I'm not sure that really helps.

@dpaoliello dpaoliello changed the title [aarch64][clang][llvm] Allow AArch64 TLB maintenance instructions to be enabled via -march [aarch64][llvm] Allow AArch64 TLB maintenance instructions to be enabled via -march Nov 19, 2024
@llvmbot llvmbot added the clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' label Nov 19, 2024
@dpaoliello dpaoliello removed clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 19, 2024
@davemgreen
Copy link
Collaborator

These instructions are being used in the hand-written assembly for a hypervisor. The hypervisor will check at runtime if the instructions are available on the current CPU before calling this code.

We try to coordinate between GCC and LLVM, to make sure we implement the same interface. Is there a reason you can't use -march=armv8.7-a as it works today, or add a .arch armv8.7-a directive? Otherwise we will need to check with GCC to make sure they are happy to implement the same features.

@dpaoliello
Copy link
Contributor Author

I'm going to close this: using the .arch directives works, so we're unblocked.

@dpaoliello dpaoliello closed this Nov 21, 2024
@dpaoliello dpaoliello deleted the tlbrmi branch November 21, 2024 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants